Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid bashism in configure script #95

Merged
merged 1 commit into from
Apr 12, 2024
Merged

Avoid bashism in configure script #95

merged 1 commit into from
Apr 12, 2024

Conversation

okapia
Copy link
Contributor

@okapia okapia commented Apr 12, 2024

Currently, the configure script is dumping a log message for the most recent commit into the middle of the output.

&> is non-standard for redirecting stdin and stderr. It is a short-hand copied from csh's >& by zsh and later bash. It is equivalent to >word 2>&1 which will also work in dash (/bin/sh on Debian) or other plain Bourne/POSIX shells.

I would perhaps question whether it might be better to check for the existence of a .git directory instead. Or for a git invocation that does less, perhaps run git rev-parse --git-dir, though that still needs the redirection. Or drop the autoconf test and handle things within the Makefile. Many projects use other forms such as git describe --tags --long --abbrev=7 instead.

@simo5
Copy link
Contributor

simo5 commented Apr 12, 2024

thanks,
if you could add DCO it'd be nice

&> is non-standard for redirecting stdin and stderr. It is a short-hand copied
from csh's >& by zsh and later bash. It is equivalent to >word 2>&1 which will
also work in dash (/bin/sh on Debian) or other plain Bourne/POSIX shells.

Signed-off-by: Oliver Kiddle <okiddle@yahoo.co.uk>
@okapia
Copy link
Contributor Author

okapia commented Apr 12, 2024

I've pushed a replacement commit with -s for the Signed-off-by trailer to be added.
I didn't find any official text linked from gssproxy (or freeipa) so I'm assuming you're just using the same text as used by Linux (at https://developercertificate.org/). Perhaps this could be made explicit somewhere. Thanks

@simo5
Copy link
Contributor

simo5 commented Apr 12, 2024

Thank you, yes github assumes the "standard" DCO as published by the Linux Kernel and we do the same.

@simo5 simo5 merged commit 69bc78f into gssapi:main Apr 12, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants